Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add missing act wrappers #7126

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Add missing act wrappers #7126

merged 2 commits into from
Dec 16, 2024

Conversation

Parsium
Copy link
Contributor

@Parsium Parsium commented Dec 11, 2024 •
edited
Loading

Proposed behaviour

  • In each RTL test, wrap all interactions that could update state with an act wrapper

Current behaviour

  • In our RTL tests, some state-updating actions aren't wrapped with act. Currently, no warnings are thrown, but they will be when RTL and React are upgraded

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Playwright automation tests added or updated if required
  • Storybook added or updated if required
  • Translations added or updated (including creating or amending translation keys table in storybook) if required
  • Typescript d.ts file added or updated if required
  • Related docs have been updated if required

QA

  • Tested in provided StackBlitz sandbox/Storybook
  • Add new Playwright test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

This PR implements essential changes required for Carbon to support React 18. These changes have been addressed now to minimise the complexity of the upcoming React 18 upgrade PR.

Testing instructions

@Parsium Parsium self-assigned this Dec 11, 2024
@Parsium Parsium force-pushed the prep-react-18 branch 2 times, most recently from f21e817 to c838369 Compare December 12, 2024 15:46
@Parsium
test: wrap interactions that update state with act wrapper
18a4012 
in preparation for consuming latest version of React Testing Library
and its sibling packages
edleeks87
edleeks87 reviewed Dec 16, 2024
View reviewed changes
Copy link
Contributor

@edleeks87 edleeks87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the QA label fyi, as we're upgrading storybook I think it's worth a brush test to ensure nothing's fallen off etc. Happy to approve when the conflicts are resolved, good work @Parsium

@Parsium Parsium changed the title Add missing act wrappers and upgrade Storybook to v8.4.7 Add missing act wrappers Dec 16, 2024
@Parsium
Copy link
Contributor Author

Parsium commented Dec 16, 2024

As React 18 will require a comprehensive QA review, I have removed the Storybook upgrade from this PR to prevent duplication of effort.

@edleeks87 edleeks87 marked this pull request as ready for review December 16, 2024 11:58
@edleeks87 edleeks87 requested a review from a team as a code owner December 16, 2024 11:58
@Parsium Parsium merged commit d1a7d79 into master Dec 16, 2024
25 checks passed
@Parsium Parsium deleted the prep-react-18 branch December 16, 2024 12:43
@carbonci
Copy link
Collaborator

🎉 This PR is included in version 144.22.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edleeks87 edleeks87 edleeks87 approved these changes

@mihai-albu-sage mihai-albu-sage mihai-albu-sage approved these changes

Assignees

@Parsium Parsium

Labels
No QA Required No UX QA Required Passed Review released Squad Front End
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Parsium @carbonci @edleeks87 @mihai-albu-sage